Catch EMS variables initialized after reference #11409

joseph-robertson · 2026-01-29T23:05:46Z

Pull request overview

Fixes Unitialized EMS variable is misused #11360

Description of the purpose of this PR

Pull Request Author

Reviewer

…ter reference does not throw error.

mitchute · 2026-01-30T01:47:14Z

As I understand it, Windows builds don't pick up uninitialized variables, which we treat as errors.

/home/mitchute/Projects/EnergyPlus/tst/EnergyPlus/unit/EMSManager.unit.cc:1065:9: error: unused variable ‘wallSurfNum’ [-Werror=unused-variable]
 1065 |     int wallSurfNum = Util::FindItemInList("WALL", state->dataSurface->Surface);
      |         ^~~~~~~~~~~

rraustad · 2026-01-30T04:04:59Z

I am using Windows 11 and do not get the unused variable warning for wallSurfNum, well, because it is initialized at line 1065 and used at 1068. If there is an issue with EMS initializing a variable before it's used (e.g., site_temp_adj) then this unit test should fail. But it doesn't, it passes with. wallSurfNum = 1, ViewFactorGround = 0.1 and seriousErrorFound = false (i.e., EXPECT_TRUE on line 1076 should be EXPECT_FASLE and line 1068 is correct). So what should happen here? I assume this unit test should check for whether site_temp_adj was initialized before it was used, but I don't see that in this unit test.

joseph-robertson · 2026-01-30T16:35:57Z

This PR is about the EMS variable site_temp_adj, not wallSurfNum. The intent (i.e., 8623d71) was to commit a failing test, to demonstrate that the uninitialized site_temp_adj should throw an error but does not. The test fails on my machine.

joseph-robertson · 2026-01-30T16:55:38Z

tst/EnergyPlus/unit/EMSManager.unit.cc

+        "    Set power_mult = site_temp_adj,                !- Program Line 1",
+        "    Set site_temp_adj = 0.1,                       !- Program Line 2",


Variable site_temp_adj is being initialized after it is referenced. We should expect an error, right?

joseph-robertson · 2026-01-30T16:56:19Z

tst/EnergyPlus/unit/EMSManager.unit.cc

+    int wallSurfNum = Util::FindItemInList("WALL", state->dataSurface->Surface);
+    bool anyRan;
+    EMSManager::ManageEMS(*state, EMSManager::EMSCallFrom::BeginTimestepBeforePredictor, anyRan, ObjexxFCL::Optional_int_const());
+    EXPECT_EQ(state->dataSurface->Surface(wallSurfNum).ViewFactorGround, 0.1);


I intentionally had this line commented out so that we could get to EXPECT_TRUE(seriousErrorFound); down below.

Sure, feel free to do what you need. I won't interfere anymore until you need someone to look. I was just pointing out that the CI linux builds were failing due to to the unused variable, that we treat as an error.

No problem, understood.

… just ManageSimulation.

joseph-robertson · 2026-01-30T22:37:54Z

Pretty sure I have this narrowed down. I think GetEMSInput can set state.dataRuntimeLang->ErlVariable(VariableNum).Value.initialized = true without calling EvaluateExpression first.

joseph-robertson · 2026-01-30T22:42:25Z

tst/EnergyPlus/unit/EMSManager.unit.cc

+
+    internalVarNum = RuntimeLanguageProcessor::FindEMSVariable(*state, "site_temp_adj", 1);
+    ASSERT_GT(internalVarNum, 0);
+    EXPECT_TRUE(state->dataRuntimeLang->ErlVariable(internalVarNum).Value.initialized);


This is the main difference with the first test; why is this suddenly initialized?

joseph-robertson · 2026-02-06T23:21:44Z

Pretty sure I have this narrowed down. I think GetEMSInput can set state.dataRuntimeLang->ErlVariable(VariableNum).Value.initialized = true without calling EvaluateExpression first.

It's not in GetEMSInput. It happens somewhere toward the beginning of ManageSimulation (likely in SetupSimulation whose purpose is to "... execute a few time steps of a simulation to facilitate setting up model ..."). I can confirm this by looking at RuntimeLanguageProcessor::FindEMSVariable(state, "site_temp_adj", StackNum); just after SetupSimulation, which shows the EMS variable is indeed initialized (yet EvaluateStack was never called for on the EMS program). Then it looks like the EMSManager::EMSCallFrom::SetupSimulation just below sets the variable back to 0.0.

I'm not totally sure what the fix is here. Perhaps EvaluateStack is being called via SetupSimulation, but with FinishProcessingUserInput=true?

…pSimulation.

joseph-robertson · 2026-02-10T23:44:42Z

testfiles/_ResidentialBase.idf

+  Output:EnergyManagementSystem,
+    Verbose,                    !- Actuator Availability Dictionary Reporting
+    Verbose,                    !- Internal Variable Availability Dictionary Reporting
+    Verbose;              !- EMS Runtime Language Debug Output Level


This test file has a program (BeginZoneTimestepAfterInitHeatBalance) that uses variables which are initialized in another program/subroutine (EndOfSystemTimestepAfterHVACReporting). I.e., initialized after reference.

joseph-robertson · 2026-02-10T23:49:59Z

testfiles/RetailPackagedTESCoil.idf

-    Run CalcBouyancyTerms,   !- <none>
-    SEt Numerator = WindMCT + NatBouyMCT + SumHATroof + QdotCond + SumReliefMCT,  !- <none>
+    Set Numerator = WindMCT + NatBouyMCT + SumHATroof + QdotCond + SumReliefMCT,  !- <none>
    Set Denominator = WindMC + NatBouyMC + SumHAroof + SumIntakeMdotCp,  !- <none>
    Set Troof = Numerator / Denominator,  !- <none>
    IF Ta < 2.0,             !- <none>
    Set Troof = Ta,          !- <none>
    ENDIF,                   !- <none>
    IF Troof < Twb,          !- <none>
    Set Twb = Troof - 0.2,   !- <none>
-    ENDIF;                   !- <none>


Something weird going on in this program. Here Troof is a function of, e.g., NatBouyMCT. Subroutine CalcBouyancyTerms needs Troof, which is used to calculate NatBouyMCT... am I missing something here?

joseph-robertson · 2026-02-10T23:53:52Z

src/EnergyPlus/RuntimeLanguageProcessor.cc

+                    if ((!state.dataGlobal->DoingSizing && !state.dataGlobal->KickOffSimulation && !state.dataEMSMgr->FinishProcessingUserInput) ||
+                        (!(thisErlVar.SetByGlobalVariable || thisErlVar.SetByInternalVariable))) {


The basic idea here is to do what we were doing before, unless the uninitialized variable is either a global/internal variable. If it isn't global/internal, set seriousErrorFound = true. If it is, let it slide.

github-actions · 2026-02-11T00:06:24Z

⚠️ Regressions detected on ubuntu-24.04 for commit `f4e0687`

Regression Summary

EDD: 2
EIO: 1
ESO Big Diffs: 1
MTR Big Diffs: 1
Table Big Diffs: 1
Table String Diffs: 1

github-actions · 2026-02-11T00:29:54Z

⚠️ Regressions detected on macos-14 for commit `f4e0687`

Regression Summary

EDD: 2
EIO: 1
ESO Big Diffs: 1
MTR Big Diffs: 1
Table Big Diffs: 1

joseph-robertson added 2 commits January 29, 2026 16:00

Add failing EMS unit test to demonstrate that variable initialized af…

8623d71

…ter reference does not throw error.

Formatting.

ead50b6

joseph-robertson self-assigned this Jan 29, 2026

fix uninitialized var

65966b8

mitchute added the Defect Includes code to repair a defect in EnergyPlus label Jan 30, 2026

joseph-robertson commented Jan 30, 2026

View reviewed changes

Add test to show difference between calling ManageEMS individually vs…

d039752

… just ManageSimulation.

joseph-robertson commented Jan 30, 2026

View reviewed changes

joseph-robertson added 8 commits February 9, 2026 10:08

Add AlwaysFindSeriousError to EMS manager data struct.

69e2e71

Update EvaluateStack to use new AlwaysFindSeriousError.

c1c2bee

Update ManageSimulation to use new AlwaysFindSeriousError around Setu…

68543c1

…pSimulation.

Clean up new EMS variable unit tests.

cf65f57

Check for global variable instead of forcing error find.

9f9c099

Check for internal variable as well.

590e0ad

Formatting in new unit test.

836fd7e

Temporary debugging changes in some testfiles.

b0bb17c

joseph-robertson commented Feb 10, 2026

View reviewed changes

		" Set power_mult = site_temp_adj, !- Program Line 1",
		" Set site_temp_adj = 0.1, !- Program Line 2",

		if ((!state.dataGlobal->DoingSizing && !state.dataGlobal->KickOffSimulation && !state.dataEMSMgr->FinishProcessingUserInput) \|\|
		(!(thisErlVar.SetByGlobalVariable \|\| thisErlVar.SetByInternalVariable))) {

Catch EMS variables initialized after reference #11409

Are you sure you want to change the base?

Catch EMS variables initialized after reference #11409

Conversation

joseph-robertson commented Jan 29, 2026

Pull request overview

Description of the purpose of this PR

Pull Request Author

Reviewer

Uh oh!

mitchute commented Jan 30, 2026

Uh oh!

rraustad commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joseph-robertson commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joseph-robertson Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

joseph-robertson Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

mitchute Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

joseph-robertson Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

joseph-robertson commented Jan 30, 2026

Uh oh!

joseph-robertson Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

joseph-robertson commented Feb 6, 2026

Uh oh!

joseph-robertson Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

joseph-robertson Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

joseph-robertson Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 11, 2026

⚠️ Regressions detected on ubuntu-24.04 for commit f4e0687

Uh oh!

github-actions bot commented Feb 11, 2026

⚠️ Regressions detected on macos-14 for commit f4e0687

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rraustad commented Jan 30, 2026 •

edited

Loading

joseph-robertson commented Jan 30, 2026 •

edited

Loading

joseph-robertson Feb 10, 2026 •

edited

Loading

⚠️ Regressions detected on ubuntu-24.04 for commit `f4e0687`

⚠️ Regressions detected on macos-14 for commit `f4e0687`